huawei-ups2000: fix broken instant command invocations and undefined Modbus behaviors. - #3604

…upstools#3603.

A serious regression was introduced during the "fightwarn" campaign that
completely broke bypass.start, shutdown.return, shutdown.reboot, and
shutdown.reboot.graceful. Any attempt to use them will fail, with an error
logged to `syslog`:

    huawei-ups2000: instcmd: command [bypass.start] reg1 is negative
    huawei-ups2000: instcmd: command [shutdown.return] reg1 is negative
    huawei-ups2000: instcmd: command [shutdown.reboot] reg1 is negative
    huawei-ups2000: instcmd: command [shutdown.reboot.graceful] reg1 is negative

Because the regression was introduced shortly after the initial driver
was merged into the upstream, these instant commands do no work in any
NUT version (with the exception of the earliest development build). This
problem was not discovered during pre-merge test for the same reason: it's
a regression that occurred after the Pull Request was merged.

In huawei-ups2000, all instant commands are driven by a lookup table to
consolidate the logic to a central dispatcher. Some commands are dispatched
to a (*handler_func)(uint16_t reg1) function, which is used to handle commands
that needs additional processing.

Not all *handler_func actually reads "reg1", it's an optional argument.
Because declaring the function to accept a variadic or a (void *) would
be over-engineering, a simple unsigned 16-bit value is used, *handler_func
may or may not use it.

During the project-wide "fightwarn" refactor campaign, it was noticed that
"reg1" is stored in the lookup table as "int16_t" (-1 indicates an unneeded
register), but *handler_func accepts a "uint16_t". In case that *handler_func
doesn't need the register ID from the lookup table, "reg1" is implicitly
converted from -1 to 65535. This is harmless, since "reg1" is not actually
used in this case.

Unfortunately, in an attempt to make the code "safe" by suppressing compiler
warning, an incorrect range check was added to the function, which refuses
to invoke *handler_func if "reg1" is negative.

As a result, all instant commands that don't use "reg1" are broken. These
commands include bypass.start, shutdown.return, shutdown.reboot, and
shutdown.reboot.graceful.

To fix this problem, fix the implicit type conversion correctly by change
the "reg1" data type in the lookup table from "int16_t" to "uint16_t", and
remove the "negative reg1" check. As defensive programming. all *handler_func
that actually need "reg1" would check and abort if "reg1" is 0.

Fixes: e9f02e2 ("drivers/huawei-ups2000.c: instcmd(): range-check and cast for ups2000_write_register() and handler_func()")
Signed-off-by: Yifeng Li <tomli@tomli.me>

…fix networkupstools#3593.

On some Huawei UPS2000 models, no instant commands or variable writes
are possible, because modbus_write_registers() doesn't work correctly.

Upon investigation, Huawei UPS2000 doesn't support multiple-register writes
at all (Modbus command 0x10), the official datasheet only supports single-
register writes (Modbus command 0x06). Multiple-register writes worked on
some models, but this turned out to be undefined behavior, it doesn't work
with all models.

This commit switches modbus_write_registers() to modbus_write_register()
to fix the problem.

Signed-off-by: Yifeng Li <tomli@tomli.me>

…networkupstools#3603.

Two bugs networkupstools#3593, networkupstools#3603 can prevent users from sending any instant
commands or changing any variables at all. Document these known
bugs in the man page.

Signed-off-by: Yifeng Li <tomli@tomli.me>

…workupstools#3593, networkupstools#3603]

Signed-off-by: Yifeng Li <tomli@tomli.me>